feat: add SRC-16 Typed Structured Data #161
Conversation
|
Thanks for the contribution! Unfortunately we can't verify the commit author(s): Antony <b***@g***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Fuel Labs Contributor License Agreement and this Pull Request will be revalidated. |
|
Thanks for the contribution! Before we can merge this, we need @CatspersCoffee to sign the Fuel Labs Contributor License Agreement. |
|
Just giving you a heads up, we will be reviewing this standard in the following days, but Fuel is enacting a code freeze around christmas as many contributors will be on vacation etc. So this will likely not be merged until January as the earliest. |
There was a problem hiding this comment.
This is amazing!
Just a few questions, primarily surrounding whether the existing encoder and hashing functionality in the std-lib and core are sufficent/can be integrated or if we need to introduce new traits and structs.
I'm also curious on how handling Fuel specific types would work. We've made the distinct choice to separate the Address and ContractId types as well as have AssetId.
| /// A demo struct representing a mail message | ||
| pub struct Mail { | ||
| /// The sender's address | ||
| pub from: b256, |
There was a problem hiding this comment.
| pub from: b256, | |
| pub from: Address, |
| pub from: b256, | |
| pub from: ContractId, |
| pub from: b256, | |
| pub from: Identity, |
To follow Sway principles, this should use either Address, ContractId, or Identity.
The same applies to to below.
There was a problem hiding this comment.
This has been adjusted in the Fuel example to reflect the Address native type.
/// A demo struct representing a mail message
pub struct Mail {
/// The sender's address
pub from: Address,
/// The recipient's address
pub to: Address,
/// The message contents
pub contents: String,
}There was a problem hiding this comment.
Looks like it was updated in examples/src16-typed-data/fuel_example/src/main.sw but not yet in examples/src16-typed-data/ethereum_example/src/main.sw 🙂 Happy to approve after these changes
bitzoic
added
the
New Standard
…SRC16Domain fuel_example contract, Adjust types for domains. Update docs.
| } | ||
|
|
||
| // EIP712 Domain (Ethereum compatible) | ||
| pub struct EIP712Domain { |
There was a problem hiding this comment.
Is this not missing a salt field?
| * Add SRC16_DOMAIN_TYPE_HASH | ||
| * Add Keccak256 hash of name string | ||
| * Add Keccak256 hash of version string | ||
| * Add chain ID as 32-byte big-endian |
There was a problem hiding this comment.
As the field in the struct is a u64, can you make it clear here that the bytes are to be padded?
|
Looking good to me; happy to approve once above comments are addressed and CI is passing |
Type of change
Changes
The following changes have been made:
SRC16Domain(Fuel) andEIP712Domain(Ethereum).Notes
Related Issues
Closes #
Checklist
New Featurelabels where relevant.